Skip to content

github_actions: pass home hostname to gh-actions-lock - #16504

Open
nodeselector wants to merge 6 commits into
dependabot:mainfrom
nodeselector:nodeselector/gh-actions-lock-hostname
Open

nodeselector wants to merge 6 commits into
dependabot:mainfrom
nodeselector:nodeselector/gh-actions-lock-hostname

Conversation

@nodeselector

@nodeselector nodeselector commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

What are you trying to accomplish?

Pass the repository's home host to gh-actions-lock so lockfile relocks stay correct with the hostname-aware CLI (github/gh-actions-lock#137).

The CLI binds every pin with an omitted lockfile hostname to its home host. Core runs it in a temp dir with no git remote, so the home host silently defaults to github.com.

Anything you want to highlight for special attention from reviewers?

Draft for staging only. The Dockerfile temporarily builds the CLI from source at c5ee15d, because no release exists for that SHA yet. It moves back to a release pin before review.

How will you know you've accomplished your goal?

Staging dry-runs against existing lockfiles. Relocks should keep the right host binding and write v0.0.3.

A FileUpdater spec checks that source_hostname: "tenant.ghe.com" reaches CliEngine.new(credentials, hostname: "tenant.ghe.com"). Parser specs keep an omitted-hostname pin on the home host while the tenant returns 503, and send an explicit github.com pin to github.com without probing the tenant. A CliEngine spec maps a transient 502 to EngineError.

Checklist

  • I have run the complete test suite to ensure all tests and linters pass.
  • I have thoroughly tested my code changes to ensure they work as expected, including adding additional tests for new functionality.
  • I have written clear and descriptive commit messages.
  • I have provided a detailed description of the changes in the pull request, including the problem it addresses, how it fixes the problem, and any relevant details about the implementation.
  • I have ensured that the code is well-documented and easy to understand.

Copilot AI balanced review requested due to automatic review settings October 8, 2026 20:43
@github-actions github-actions Bot added the L: github:actions GitHub Actions label Oct 8, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

馃煛 Changes recommended

The dry-run path still defaults tenant relocks to github.com, and the staging-only source build must be replaced before merging.

3 open findings
What changed in this PR

Adds hostname-aware GitHub Actions lockfile relocking, including *.ghe.com support and schema v0.0.3 compatibility.

Changes:

  • Propagates the repository hostname and selects only its credential.
  • Enables experimental *.ghe.com lockfile handling and maps host errors.
  • Temporarily builds the hostname-aware CLI from source.
File Description
updater/鈥媠pec/鈥媎ependabot/鈥媎ependency_change_builder_spec.rb Tests hostname option propagation.
updater/鈥媗ib/鈥媎ependabot/鈥媎ependency_change_builder.rb Passes the source hostname to file updaters.
github_actions/鈥媠pec/鈥媎ependabot/鈥媑ithub_actions/鈥媗ockfile/鈥媣ersion_gate_spec.rb Covers schema v0.0.3.
github_actions/鈥媠pec/鈥媎ependabot/鈥媑ithub_actions/鈥媗ockfile/鈥媏nv_spec.rb Tests tenant credential selection.
github_actions/鈥媠pec/鈥媎ependabot/鈥媑ithub_actions/鈥媗ockfile/鈥媍li_engine_spec.rb Tests hostname arguments and identity errors.
github_actions/鈥媠pec/鈥媎ependabot/鈥媑ithub_actions/鈥媐ile_fetcher_spec.rb Covers experimental ghe.com fetching.
github_actions/鈥媗ib/鈥媎ependabot/鈥媑ithub_actions/鈥媗ockfile/鈥媣ersion_gate.rb Supports multiple schema versions.
github_actions/鈥媗ib/鈥媎ependabot/鈥媑ithub_actions/鈥媗ockfile/鈥媏rrors.rb Updates supported-version messaging.
github_actions/鈥媗ib/鈥媎ependabot/鈥媑ithub_actions/鈥媗ockfile/鈥媏nv.rb Selects credentials for the home host.
github_actions/鈥媗ib/鈥媎ependabot/鈥媑ithub_actions/鈥媗ockfile/鈥媍li_engine.rb Supplies hostname and maps identity failures.
github_actions/鈥媗ib/鈥媎ependabot/鈥媑ithub_actions/鈥媐ile_updater.rb Forwards hostname during relocking.
github_actions/鈥媗ib/鈥媎ependabot/鈥媑ithub_actions/鈥媐ile_fetcher.rb Gates *.ghe.com lockfiles experimentally.
github_actions/鈥媗ib/鈥媎ependabot/鈥媑ithub_actions/鈥媍onstants.rb Adds schema v0.0.3 support.
github_actions/鈥婦ockerfile Builds the unreleased CLI revision from source.

馃 Review effort: Balanced


馃挕 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread github_actions/Dockerfile Outdated
Comment thread github_actions/lib/dependabot/github_actions/file_updater.rb
Comment thread updater/spec/dependabot/dependency_change_builder_spec.rb
Builds github/gh-actions-lock#137 at c5ee15d from source; no release
exists for this SHA. Swap back to a release pin before review.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 8, 2026 21:41
@nodeselector
nodeselector force-pushed the nodeselector/gh-actions-lock-hostname branch from 743c5f2 to 8d124a5 Compare October 8, 2026 21:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

馃煛 Changes recommended

The Dockerfile still contains the explicitly temporary source build that must return to a checksummed release pin before merge.

1 open finding
2 resolved since last review

馃 Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 8, 2026 21:50
@nodeselector
nodeselector force-pushed the nodeselector/gh-actions-lock-hostname branch from 8d124a5 to 9050ba9 Compare October 8, 2026 21:50
@nodeselector
nodeselector marked this pull request as ready for review October 8, 2026 21:53
@nodeselector
nodeselector requested a review from a team as a code owner October 8, 2026 21:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

馃煛 Changes recommended

The FileUpdater hostname handoff lacks direct regression coverage, and the Dockerfile remains staging-only.

1 open finding
1 resolved since last review

馃 Review effort: Balanced

# Materialize the full onboarded closure so the lock remains intact, but fix
# only changed workflows so unrelated refs are not touched.
content = Lockfile::CliEngine.new(credentials).relock(
content = Lockfile::CliEngine.new(credentials, hostname: source_hostname).relock(
nodeselector and others added 2 commits October 8, 2026 15:02
The engine binds every pin with an omitted lockfile hostname to its home
host. It runs in a temp dir with no git remote, so without --hostname
the home host silently defaults to github.com. Pass --hostname from the
job source and forward only a home-host token. Accept the v0.0.3 schema
the engine now writes, and report lock host-identity failures as
user-actionable rather than engine errors.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Fetch actions.lock for *.ghe.com sources under the existing
github_actions_lockfile experiment, same as github.com.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 8, 2026 22:07
@nodeselector
nodeselector force-pushed the nodeselector/gh-actions-lock-hostname branch from 9050ba9 to 9b6de3b Compare October 8, 2026 22:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

馃煛 Changes recommended

The Dockerfile still contains the explicitly temporary source build that must return to a checksummed release pin before merging.

1 open finding

馃 Review effort: Balanced

@v-kbukum1 v-kbukum1 self-assigned this Oct 9, 2026
@v-kbukum1

Copy link
Copy Markdown
Contributor

Before merge

Worth double-checking

  • Error mapping (lockfile/cli_engine.rb:32): upstream, verifying repository identity also wraps transient errors such as 5xx, 429 and timeouts. Please make sure those don't surface to users as dependency_file_not_resolvable.
  • Deleted workflows (file_updater.rb:85): relocks are scoped to the changed workflows, so pins for removed workflows can stay in the lockfile. On Proxima, VerifyHosts checks every pin. Please confirm that a stale pin can't fail every relock.
  • Lockfile hostname (file_parser.rb:139-146): the parser resolves tenant-first and doesn't read the pin's hostname, while the CLI does. Please confirm this is OK for now, or track it as a follow-up.
  • Lockfile format: relocks write v0.0.3, which older CLI versions (v0.1.6) reject. Please make sure users have a compatible CLI before prod.

If we want to do dry-run for proxima

  • bin/dry-run.rb doesn't pass source_hostname, so Proxima dry-runs default to github.com.

CI: the nuget and pip-compile failures are unrelated.

gh-actions-lock wraps every RepoIDs error in 'verifying repository
identity', so 5xx, rate-limit 403s and timeouts were surfacing as
dependency_file_not_resolvable instead of retryable engine errors.
A pinned action now uses the host recorded in the lockfile (omitted
means the home host) instead of probing the tenant and falling back to
github.com on any failure.
Copilot AI balanced review requested due to automatic review settings October 9, 2026 21:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

馃煛 Changes recommended

Legacy v0.0.2 pins are incorrectly rebound from github.com to the tenant home host.

2 open findings

馃 Review effort: Balanced

Comment on lines +77 to +78
hostname = entry["hostname"].to_s
hostname.empty? ? home : hostname
`$/` composites can live anywhere in the repo and call each other, so the
engine needs every action.yml/action.yaml at its repo-relative path. Fetch
them with one recursive tree call when a workflow uses `$/` and the lock
exists, and name the temp dir as the repo root via GH_ACTIONS_LOCK_REPO_ROOT.
Copilot AI balanced review requested due to automatic review settings October 12, 2026 06:52
# relocking needs every action definition, at its repo-relative path.
sig { returns(T::Array[DependencyFile]) }
def self_repository_action_files
return [] unless correctly_encoded_workflow_files.any? { |f| f.content&.match?(SELF_REPOSITORY_USES) }

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

馃煛 Changes recommended

Hostname lookup conflates case-sensitive refs, while local-action discovery is unbounded and incomplete for truncated repository trees.

3 open findings
Files not reviewed (1)
  • sorbet/rbi/shims/github_with_retries.rbi: File type not supported
Previously missed (1)

In code that hasn't changed since last review

Medium severity Handle case-sensitive refs and non-string lock keys safely

github_actions/鈥媗ib/鈥媎ependabot/鈥媑ithub_actions/鈥媗ockfile/鈥媟eader.rb:74

This compares the entire lock key case-insensitively, but Git refs are case-sensitive. A pin for owner/repo@Release can therefore supply the hostname for owner/repo@release, even though those may resolve to different refs. It also raises on a syntactically valid malformed lock with a non-string dependency key. Compare only the repository portion case-insensitively and the ref exactly, while ignoring non-string keys.

馃 Review effort: Balanced

Comment on lines +101 to +104
repo_action_paths
.reject { |path| correctly_encoded_workflow_files.any? { |f| f.path == path } }
.map do |path|
file = fetch_file_from_host(Pathname.new(path).relative_path_from(File.join("/", directory)).to_s)

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L: github:actions GitHub Actions

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

4 participants